fix(wc-memberships): skip expiry filter for non-subscription memberships#4743
Conversation
The wc_memberships_expire_user_membership filter fires for every user membership, not just subscription-tied ones. The handler assumed a \WC_Memberships_Integration_Subscriptions_User_Membership and called get_subscription_id() on it; plain WC_Memberships_User_Membership objects don't have that method, so the call threw an Uncaught Error. Bail out early via method_exists() and widen the docblock type to the base class.
There was a problem hiding this comment.
Pull request overview
Fixes a fatal error in Membership_Expiry::prevent_membership_expiration() when the wc_memberships_expire_user_membership filter fires for non-subscription-tied memberships, which don't expose get_subscription_id().
Changes:
- Adds a
method_exists()guard to early-return when the membership object lacksget_subscription_id(). - Widens the docblock type of
$user_membershipfrom the Subscriptions integration class to the base class. - Adds a unit test that reproduces the original fatal and verifies pass-through behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| includes/plugins/wc-memberships/class-membership-expiry.php | Guards against non-subscription memberships and updates docblock. |
| tests/unit-tests/plugins/wc-memberships/membership-expiry.php | New unit test verifying the filter passes through for non-subscription memberships. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
🎉 This PR is included in version 6.40.0-hotfix-membership-expiry-non-subscription-guard.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
adekbadek
added
the
[Status] Needs Review
jason10lee
left a comment
jason10lee
left a comment
There was a problem hiding this comment.
Approved! Targeted fix, tests clean, even adds to the test suite. (Thank you!)
github-actions
Bot
added
[Status] Approved
|
Hey @adekbadek, good job getting this PR merged! 🎉 Now, the Please check if this PR needs to be included in the "Upcoming Changes" and "Release Notes" doc. If it doesn't, simply remove the label. If it does, please add an entry to our shared document, with screenshots and testing instructions if applicable, then remove the label. Thank you! ❤️ |
All Submissions:
Changes proposed in this Pull Request:
Membership_Expiry::prevent_membership_expiration()is hooked intowc_memberships_expire_user_membership, which fires for every user membership. The handler assumed every membership was a subscription-tied one (\WC_Memberships_Integration_Subscriptions_User_Membership) and calledget_subscription_id()on it. For plainWC_Memberships_User_Membershipobjects (no linked subscription), that method does not exist, so the call throwsUncaught Error: Call to undefined method WC_Memberships_User_Membership::get_subscription_id().This adds an early
method_exists()guard that returns the incoming$cancel_membershipvalue unchanged when the membership is not subscription-tied, so the filter is effectively a no-op for memberships it cannot reason about. The docblock type for$user_membershipis also widened from the integration class to the base class to match reality.How to test the changes in this Pull Request:
wp wc memberships .... Confirm no PHP fatal is logged.composer test -- --filter test_passes_through_non_subscription_membership. It must pass; without the fix it errors withCall to undefined method ... ::get_subscription_id().composer test.Other information: